-
Notifications
You must be signed in to change notification settings - Fork 447
feat: Instantiation payload support for INetworkPrefabInstanceHandler #3430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop-2.0.0
Are you sure you want to change the base?
Conversation
…efabInstanceHandler.Instantiate() documentation
I just posted a video demonstrating how the system works:
In the video, I spawn The
2025-04-27.21-45-44.mp4By implementing a custom Here is the core implementation: public class TestHandlerDeterministicLink : INetworkPrefabInstanceHandler, INetworkInstantiationPayloadSynchronizer
{
public Dictionary<int, DeterministicIDHolder> deterministicSpawns = new Dictionary<int, DeterministicIDHolder>();
public int customSpawnID = 0;
void INetworkInstantiationPayloadSynchronizer.OnSynchronize<T>(ref BufferSerializer<T> serializer) => serializer.SerializeValue(ref customSpawnID);
public NetworkObject Instantiate(ulong clientId, Vector3 position, Quaternion rotation)
{
var obj = deterministicSpawns[customSpawnID];
TMP_Text text = obj.GetComponent<TMP_Text>();
text.SetText(text.text + "*");
return obj.GetComponent<NetworkObject>();
}
public void Destroy(NetworkObject networkObject) => GameObject.Destroy(networkObject.gameObject);
int nextDeterministicId = 0;
public void InstantiateLocally(GameObject linkablePrefab)
{
var spawned = GameObject.Instantiate(linkablePrefab);
spawned.transform.position = UnityEngine.Random.insideUnitCircle * 0.01f;
var text = spawned.GetComponent<TMP_Text>();
text.SetText(nextDeterministicId.ToString() + "&" + text.text);
var deterministicIdHolder = spawned.GetComponent<DeterministicIDHolder>();
deterministicSpawns[nextDeterministicId] = deterministicIdHolder;
deterministicIdHolder.SetID(nextDeterministicId);
nextDeterministicId++;
}
} Warning While this system enables advanced workflows, |
This would actually save us a lot of trouble. Right now when after we spawn stuff we have to run like two or three RPCs just to finish setting up objects properly. I wish you luck on get it merged on Unity 6.1, it would be super useful for our current project. |
Sure! I'm just waiting for reviewers to be assigned to this PR. |
I just got more feedback on the Issue #3421 I'm thinking about a way to have the prefab handler "write" the payload right before the spawn happens. In this approach, I would try to move most of the logic into CreateObjectMessage, removing it from the object data. This would avoid all the newly added generics and any potential object boxing. I'm converting this PR into a draft to keep modifying the implementation and will get back to comment once it's ready for feedback again. |
[Sorry bad writting i might edit this text later] I did requested changes by @EmandM into the PR, currently im reusing the same buffer serializer from the object serialization. Also i could make the new interface to not have the OnSynchronize, and having instead Serialize and Deserialize methods, but that would make the usage not as comfortable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fantastic next step! The video was super helpful to understand what the intention was. Is there any chance you have a more minimalistic example of this feature that doesn't require linking the items together later. The added complexity of having separate objects that are linked implies that this feature is doing the linking. My understanding is simply that this feature enables changing the object that is instantiated as part of the instantiation flow.
A few notes on the code:
Out of interest, is there a reason you chose to implement the custom data passing only on scene objects? We'd prefer a solution that works everywhere where the prefab handler can work. Again, the symmetry in the approach is important. If you can do something with prefab handlers in one area, that feature should work in all areas.
It would also be fantastic if you can add a test showing how this feature is used.
/// Interface for synchronizing custom instantiation payloads during NetworkObject spawning. | ||
/// Used alongside <see cref="INetworkPrefabInstanceHandler"/> to extend instantiation behavior. | ||
/// </summary> | ||
public interface INetworkInstantiationPayloadSynchronizer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name could be more descriptive. How about something like INetworkPrefabInstantiationHandler
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this interface doesn't handle instantiation itself, that’s entirely the role of INetworkPrefabInstanceHandler
.
The purpose of INetworkInstantiationPayloadSynchronizer
is strictly to serialize and deserialize the instantiation payload before Instantiate() is called. That’s why I went with a name that emphasizes its function in data synchronization, rather than suggesting it’s involved in the instantiation logic directly.
If we go with the OnPreInstantiate
or OnBeforeInstantiation
naming you suggested, perhaps something like INetworkPrefabPreInstantiationHandler
or INetworkPrefabBeforeInstantiationHandler
would better reflect the purpose. I’m happy to update the name as long as it clearly communicates what the interface does.
I used the term payload since it directly refers to the custom data being passed along with the spawn message, which is exactly what this interface handles
I'll explore a few alternative naming options in tomorrow commit to see if any feel like a better fit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Along with this note, I was also thinking it might be nice if this new interface extends from the base interface.
So rather than needing
class MyHandler : INetworkPrefabInstanceHandler, INetworkInstantiationPayloadSynchronizer
It could instead be used as
class MyHandler : INetworkPrefabPayloadHandler
or something of that type. Keeps it clearer for developers to implement and simpler to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pretty good idea, i like how it looks simpler now, something like INetworkPefabInstanceHandlerWithData
would be similar to the old namings of the job system, like IJobComponent IJobWithECB and similar.
INetworkPrefabPayloadHandler
looks good to me but I feel INetworkPefabInstanceHandlerWithData
its even more descriptive for developers who will use it.
What do you think, could we name it INetworkPefabInstanceHandlerWithData
?
Im sure we can find a better naming later for the second interface it wraps.
/// allowing you to cache or prepare information needed during instantiation. | ||
/// </summary> | ||
/// <param name="serializer">The buffer serializer used to read or write custom instantiation data.</param> | ||
void OnSynchronize<T>(ref BufferSerializer<T> serializer) where T : IReaderWriter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid confusion with NetworkBehaviours, could we rename this to something more descriptive like OnInstantiation()
or OnBeforeInstantiation()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me! As long as the name clearly reflects that this method is solely responsible for synchronizing data prior to instantiation, I’m happy to update it.
OnBeforeInstantiation works well for that, as long as we keep in mind that it is actually serializing and deserializing data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might rename the interface to something like:
INetworkPrefabPreInstantianceDataSerializer
INetworkPrefabPreInstantianceSynchronizer
INetworkPrefabPreInstantiantiateHandler
And the method could be:
OnPreInstanceSerialization
, SynchronizePreInstance
, PreInstanceSynchronization
, HandlePreInstantiateData
This way, the naming makes it clearer that its about the data flow before instantiation, not the instantiation itself, and it also avoids confusion with NetworkBehaviour.OnSynchronize()
Let me know if any of these seem closer to NGO’s naming conventions, or if you would prefer a shorter variation
Regarding this, I've tested it, and in the video only one object is an in-scene placed object. The rest are dynamically instantiated through the prefab instance handler, not just scene objects. Maybe I misunderstood what you meant? I’ll work on a simpler example, though to be honest, the linking case is the most valuable use case I’ve found so far, its actually what motivated this feature in the first place. Right now I dont have many alternative examples because most of the benefit comes from enabling that exact flow: having objects pre-created and deterministically selected or connected based on payload metadata. Of course, it also opens the door to more advanced use cases, like sending special setup/configuration data before instantiation (For example in the video that sets up In a way, I don’t yet fully know the limits of the feature, I just know it unlocks workflows that weren’t previously possible. About the other changes, I will answer these and also come with some changes you might like tomorrow. |
I had another pass today. Definitely agree with what you've said about I also took a bit more time with the example. I absolutely see what you're doing there. Thank you for the detailed explanations. It'll be best if the function naming we go with follows the Mixing and matching from your naming options, what do you think of something like these two options? /// Option A
INetworkPrefabInstanceWithDataHandler
OnPreInstantiate()
// Option B
INetworkPrefabWithSynchronizeHandler
OnPrefabSynchronize() |
No problem, I’ll make that change in a few minutes!
Its a perfect idea, making it easier to use for developers. The next commit will include that.
Since it’s still an In contrast, Would this option work for you? public interface INetworkPrefabInstanceHandlerWithData : INetworkPrefabInstanceHandler
{
void OnSynchronizeInstantiationData<T>(ref BufferSerializer<T> serializer) where T : IReadWrite
} The method name Let me know what you think. I’ll go ahead and push a commit with these changes in the meantime and await your feedback. 😄 |
1) Moved the payload deserialization into the AddSceneObject, for deferred instantiation compatibility 2) Changed the new interface to be a direct single extended implementation, instead a complement of the handler 3) Some renames to better match what the feature does for developers
All requested changes have been implemented.
This is the same example shown earlier, but simplified and updated to reflect the new interface and naming conventions: public class TestHandlerDeterministicLink : INetworkPrefabInstanceHandlerWithData
{
Dictionary<int, DeterministicIDHolder> deterministicSpawns = new Dictionary<int, DeterministicIDHolder>();
int nextDeterministicId = 0;
int customSpawnID = 0;
public void OnSynchronizeInstantiationData<T>(ref BufferSerializer<T> serializer) where T : IReaderWriter
{
serializer.SerializeValue(ref customSpawnID);
}
public NetworkObject Instantiate(ulong clientId, Vector3 position, Quaternion rotation)
{
return deterministicSpawns[customSpawnID].GetComponent<NetworkObject>();
}
public void Destroy(NetworkObject networkObject) => GameObject.Destroy(networkObject.gameObject);
public void DoSpawn(GameObject linkablePrefab)
{
var deterministicIdHolder = GameObject.Instantiate(linkablePrefab).GetComponent<DeterministicIDHolder>();
deterministicSpawns[nextDeterministicId] = deterministicIdHolder;
deterministicIdHolder.SetID(nextDeterministicId);
nextDeterministicId++;
}
} Marking the pull request as ready for review ✅ |
Solves Issue #3421
Related to the discussions in Pull Request #3419 and Issue #3421 (follow-up proposal based on new approach).
This PR introduces support for sending custom instantiation payloads, allowing
INetworkPrefabInstanceHandler
to receive metadata before callingInstantiate()
.The feature is fully optional, backward-compatible, and requires no changes to existing user workflows.
Changelog
INetworkInstantiationPayloadSynchronizer
interface to synchronize custom pre-instantiation data.Testing and Documentation
INetworkPrefabInstanceHandler.Instantiate()
summary updated to mentionINetworkInstantiationPayloadSynchronizer
INetworkInstantiationPayloadSynchronizer
.Deprecated API
Backport
Implementation Example
Spawning flow:
Important
When spawning, you must update the handler's data before calling
Spawn()
orInstantiateAndSpawn()
.The data set in the handler will be serialized automatically during the prior the instantiation process.
Highlights